Reject non-function bindings for single-case and partial active pattern names#19763
Reject non-function bindings for single-case and partial active pattern names#19763Copilot wants to merge 4 commits into
Conversation
Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/588bbe30-9352-45a1-8895-c59b79395ad3 Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
The PR's new validation rejects non-function active pattern bindings for single-case and partial patterns (extending the existing multi-case check). The RelaxWhitespace2.fs test file had several active patterns defined as non-function values (|C|, |F|_|, |G|_|, etc.) since it tests whitespace relaxation, not active pattern semantics. Add parameters to make these active patterns function-typed while preserving the multiline whitespace structure being tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
❗ Release notes required
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
The fix is correct and well-scoped. Removing the when _apinfo.ActiveTags.Length > 1 guard causes doesActivePatternHaveFreeTypars to be called for all active pattern bindings, which triggers the existing FS1209 side-effect check (activePatternIdentIsNotFunctionTyped) for single-case and partial patterns too, while FS1210 (free typars) remains restricted to multi-case via the if condition.
Correctness — verified that stripFunTy on a non-function type returns ([], ty) with no spurious free-typar error, so the error cascade is clean.
Tests — the RelaxWhitespace2.fs fixups make sense: those active patterns were never valid for pattern matching, and making them function-typed aligns with the test's real purpose (whitespace parsing, not active-pattern semantics).
Minor note (pre-existing, not blocking): doesActivePatternHaveFreeTypars reports FS1209 as a side effect despite its name suggesting a pure predicate. The same function is called from PatternMatchCompilation.fs at use-sites (lines 651, 720, 1415, 1623), meaning a non-function active pattern can get a duplicate FS1209 at each use. This was already the case for multi-case patterns before this PR, so it's not a regression — just something to be aware of for a future cleanup pass that separates the validation concern from the predicate.
The compiler previously rejected non-function multi-case active patterns (
(|A|B|)) but still allowed equivalent invalid forms for single-case ((|A|)) and partial ((|A|_|)) names. This left declarations that tooling recognized as active patterns but could not be used in pattern positions.Checker behavior aligned across active pattern forms
Active pattern '...' is not a function) consistently apply to single-case and partial declarations as well.Conformance coverage expanded
E_ActivePatternNotAFunction.fswith explicit single-case and partial non-function declarations.Named.fsto assert all three FS1209 errors.Example of newly-rejected declarations